(Lit) Style widgets using CSS variables.#1079
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a centralized theme injection system for A2UI components, adding a default style utility for global CSS variables and an injection function. Components across the basic and minimal catalogs are updated to use these variables and call the injection function in their lifecycle. Feedback identifies a missing theme injection in the basic Text component, suggests using internal CSS variables for better color inheritance in buttons, and recommends guarding document access to ensure compatibility with server-side rendering.
ditman
force-pushed
the
core-styling-all-widgets
branch
3 times, most recently
from
903d6c6 to
3edb33b
Compare
ditman
changed the title
|
I'll address Gemini comments next |
ditman
force-pushed
the
core-styling-all-widgets
branch
from
c1ebcb5 to
2dcc0ca
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request re-styles the v0_9 catalog components in the Lit renderer to utilize a new default theme provided by web_core. This involved introducing a new injectBasicCatalogStyles function in web_core that defines CSS variables for theming, including light/dark mode support. All v0_9 Lit components were updated to use static styles = css blocks and connectedCallback to inject these new styles, replacing previous inline styling and redundant div wrappers. The package.json and package-lock.json files were updated to reflect version bumps and new peer dependencies, and the CHANGELOG.md files were also updated. Review comments noted a breaking change in the Icon component's class name, an invalid CSS border property in CheckBox, a potential visual bug with the Divider component's content, an inconsistent hover color for the primary button, and an unidiomatic Lit pattern in the Image component.
| background-color: var(--a2ui-color-secondary-hover, #ddd); | ||
| } | ||
| .a2ui-button.a2ui-button-primary:hover { | ||
| background-color: var(--a2ui-color-primary-hover, #fbd); |
There was a problem hiding this comment.
The fallback color #fbd (pink) for the primary button hover state is inconsistent with the primary blue theme (#17e). Consider using a color that aligns with the theme, such as a darker or lighter shade of blue.
| background-color: var(--a2ui-color-primary-hover, #fbd); | |
| background-color: var(--a2ui-color-primary-hover, #35f); |
renderers/lit/src/v0_9/catalogs/basic/components/AudioPlayer.ts
Outdated
Show resolved
Hide resolved
ditman
force-pushed
the
core-styling-all-widgets
branch
from
2b7611b to
ad16d84
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the v0.9 Lit catalog components to extend a new base class, BasicCatalogA2uiLitElement, which centralizes style injection and flex property management. The components now utilize Lit's static styles and CSS variables for enhanced customization, while the minimalCatalog has been deprecated and removed. Review feedback identifies mismatched CSS variable names in the Image component, redundant style logic in Column and Row that is already handled by the base class, and a missing default value for the CheckBox border variable.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Lit v0.9 basic catalog components by introducing a BasicCatalogA2uiLitElement base class to handle style injection and flex-weight properties. All components are migrated to use encapsulated static styles with CSS variables for theming, and the minimal catalog is removed in favor of the basic catalog. Feedback includes addressing a layout issue with horizontal dividers, ensuring the flex property is reset when weight is undefined, fixing inconsistent hover colors and CSS variable documentation, and simplifying class assignments.
…, li, blockquote and pre
… functionality, like automagically injecting CSS styles, and handling the weight property
…c catalog instead.
ditman
force-pushed
the
core-styling-all-widgets
branch
from
2e933c2 to
ca590b6
Compare
|
Landing this, it's getting unwieldy to maintain all the dependent PRs I have after this one. |
2 participants
Description
This PR adds styling based on CSS variables to the components of the basic catalog of the Lit renderer.
The PR uses the base styles defined in web_core, and adds per-component overrides that can be used to customize the appearance of each component further.
This PR continues #1057.
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.